Fix #9642: fix bootstrapping on Windows #9644
Conversation
liufengyun
force-pushed
the
fix-9642
branch
3 times, most recently
from
44a4158 to
598fcd2
Compare
This reverts commit a34b7b8.
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9644/ to see the changes. Benchmarks is based on merging with master (757e431) |
`dirPathTrailingSlash` is used both in `DirectoryPath` and `ZipArchiveFileLookup`, the former is platform-depndent, while the latter always uses `/` as separator. Previously, the code works by accident: `new JFile(dir, path)` and `dir.resolve(path)` are able to handle `path = "java/lang"` on windows.
| if (java.io.File.separatorChar == '/') | ||
| dirPathTrailingSlashJar | ||
| else | ||
| FileUtils.dirPath(dottedString) + java.io.File.separator |
There was a problem hiding this comment.
I don't know if this is too cautious. One justification for the duplication is to save hours of debugging for future maintainers.
WDYT @sjrd ?
There was a problem hiding this comment.
If this file name is used for lookup only, then it can be / on all platforms, including Windows. Java can find files that you specify with / as if they were \.
Where it breaks down is when listing files, then comparing their paths to some other data where you always have /, or if you split('/'). Because Windows will give you \, which won't be compared/split correctly.
There was a problem hiding this comment.
Yes, that is the worry I have. String paths like a/b/c on windows are mines: if they are accidentally spliced to form a bigger string path, errors are guaranteed. Currently (before this commit), they already leak to DirectoryPath, and works accidentally.
Fix #9642: fix bootstrapping on Windows